Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation: action, message, service typesupport and message bounds #472

Merged
merged 7 commits into from
May 5, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Apr 22, 2020

Documentation action, message, service typesupport and message bounds

Signed-off-by: ahcorde [email protected]

@ahcorde ahcorde added help wanted Extra attention is needed documentation labels Apr 22, 2020
@ahcorde ahcorde self-assigned this Apr 22, 2020
Copy link
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not bad, sorry I didn't see this review. But it also looks like there was some file naming refactoring that occurred since this PR was made so that will also need to be addressed.

@ahcorde ahcorde requested a review from brawner April 28, 2020 10:10
@dirk-thomas
Copy link
Member

The string functions still lack docblocks.

@ahcorde ahcorde requested a review from brawner April 29, 2020 15:58
Signed-off-by: ahcorde <[email protected]>
@ahcorde ahcorde force-pushed the ahcorde/docblock/rosidl_runtime_c branch from a3856a1 to 0487a6f Compare April 29, 2020 16:29
Copy link
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of nits

Signed-off-by: ahcorde <[email protected]>
@@ -34,6 +34,7 @@ extern "C"
* All strings must be null-terminated.
* The rosidl_runtime_c__String structure should be deallocated using the given function
* rosidl_runtime_c__String__fini() when it is no longer needed.
* Call this function with an already initialized string will leak memory.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Call this function with an already initialized string will leak memory.
* Calling this function with an already initialized string will leak memory.

Fix here and below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed fe40059

Signed-off-by: ahcorde <[email protected]>
@ahcorde ahcorde requested a review from dirk-thomas May 1, 2020 15:20
@ahcorde ahcorde merged commit 7528869 into master May 5, 2020
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/docblock/rosidl_runtime_c branch May 5, 2020 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants